Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaa6b03cab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js
Outdated
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/export-helpers/run-properties-export.js
Outdated
Show resolved
Hide resolved
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@VladaHarbour clean approach — tracking inline keys to avoid exporting inherited styles makes sense.
two things to fix: the style chain merge is in the wrong order (base overwrites derived), and the cell property filter drops user edits that weren't in the original import. also worth checking whether existing collab docs could have null inline keys — if so, export would strip their formatting.
minor cleanup: runPropertiesStyleKeys is saved but never read on export. run-properties-export.js has no unit tests. left inline comments.
packages/super-editor/src/core/super-converter/export-helpers/run-properties-export.js
Outdated
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js
Outdated
Show resolved
Hide resolved
packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js
Outdated
Show resolved
Hide resolved
packages/super-editor/src/core/super-converter/v3/handlers/w/r/r-translator.js
Show resolved
Hide resolved
VladaHarbour
force-pushed
the
sd-1919_filter-inherited-props
branch
from
095867f to
68bf5ce
Compare
VladaHarbour
force-pushed
the
sd-1919_filter-inherited-props
branch
from
68bf5ce to
91bad8b
Compare
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@VladaHarbour all five round 1 items fixed, nice. the collab guard covers editing, but the export itself still doesn't handle old docs where the new fields are missing — formatting gets dropped on save. same at the paragraph level. left inline comments with a fix.
|
|
||
| // Export run properties that were inline or that override the style (so user overrides are preserved). | ||
| // Exclude keys that are style-only (in styleKeys but not in overrideKeys). | ||
| const candidateKeys = [...new Set([...(inlineKeys || []), ...(overrideKeys || [])])]; |
There was a problem hiding this comment.
the collab guard you added helps when users edit text, but if someone opens an old doc and saves without editing anything, inlineKeys is still null here — so nothing gets exported. all formatting gone.
| const candidateKeys = [...new Set([...(inlineKeys || []), ...(overrideKeys || [])])]; | |
| const candidateKeys = inlineKeys != null | |
| ? [...new Set([...(inlineKeys || []), ...(overrideKeys || [])])] | |
| : Object.keys(runProperties); |
| // Only include w:rPr in pPr when the paragraph had inline rPr on import; filter to inline keys and drop if empty. | ||
| const inlineKeys = paragraphProperties.runPropertiesInlineKeys; | ||
| delete paragraphProperties.runPropertiesInlineKeys; | ||
| if (!Array.isArray(inlineKeys) || inlineKeys.length === 0) { |
There was a problem hiding this comment.
same thing for paragraphs — old docs hit this and lose their default formatting on save.
| const hadInlineKeys = | ||
| Array.isArray(runNode.attrs?.runPropertiesInlineKeys) && runNode.attrs.runPropertiesInlineKeys.length > 0; | ||
| if (JSON.stringify(runProperties) === JSON.stringify(runNode.attrs.runProperties) && hadInlineKeys) return; | ||
| const newInlineKeys = [...new Set([...existingInlineKeys, ...keysFromMarks(segments[0])])]; |
There was a problem hiding this comment.
the guard works for the early-return case, but keysFromMarks only finds mark-based properties. things like character styles, language, and text direction are in runProperties but never get added to the allow-list — so they're dropped on export.
2 participants
Exclude inherited props from document.xml on export